Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix-dio-query-parameters #239

Merged
merged 1 commit into from
Nov 10, 2024

Conversation

EArminjon
Copy link
Contributor

@EArminjon EArminjon commented Sep 19, 2024

options.queryParameters is used by Dio to offer an entrypoint for developpers to give query parameters easily.

options.queryParameters didn't contain queryParameters put inside path string :

Dio().get(
  "https://test.com/home?test=youWillNotSeeMe", 
  queryParameters: {
    "test2: youWillSeeMe",
  },
);

uri.queryParameters contain both (path.queryParameters and options.queryParameters). This PR fix that and this issue : #237

@EArminjon
Copy link
Contributor Author

Hello @jhomlala,

Can you check this PR ? It's very important for us, else we can't use the official latest version

@techouse
Copy link
Collaborator

@EArminjon can you fix your formatting?

@EArminjon
Copy link
Contributor Author

EArminjon commented Nov 10, 2024

@techouse i didn't understand how the format can be wrong :

image

@techouse
Copy link
Collaborator

Yeah, it's weird. Locally all analyzer tests pass. Can you maybe re-run the action?

@techouse
Copy link
Collaborator

techouse commented Nov 10, 2024

Ah, I see. It's an unrelated file packages/alice/lib/ui/calls_list/page/alice_calls_list_page.dart that causes the error.

@EArminjon Can you please run melos format && melos analyze and push all the changes?

@EArminjon
Copy link
Contributor Author

In final, a rebase was just needed.

OpenfileX used on my branch was the issue : they put a breaking change using 'patch' version instead of 'minor'. Thats lead to the issue above when running pub get. (A nullable string was converted to non nullable)

@techouse techouse merged commit 3311b56 into jhomlala:master Nov 10, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants